-
-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce ext.dota2
- Dota 2 Game Coordinator extension
#460
base: main
Are you sure you want to change the base?
Conversation
Done with `protoc -I . --python_betterproto_out=protos dota_gcmessages_client_watch.proto` using 1.2.5 betterproto and manual edits
only introduces one method `fetch_top_source_tv_games`
for more information, see https://pre-commit.ci
ext.dota2
- Game 2 Game Coordinator extensionext.dota2
- Dota 2 Game Coordinator extension
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for this!
These don't need to be included, they are for steam non-sense.
Also if you wouldn't mind please could you clean up the enum names to remove underscores and normalise them to be PascalCase.
Finally could you add a very barebones test to the tests directory just to ensure that the client is importable
Co-authored-by: James Hilton-Balfe <gobot1234yt@gmail.com>
for more information, see https://pre-commit.ci
Co-authored-by: James Hilton-Balfe <gobot1234yt@gmail.com>
for more information, see https://pre-commit.ci
Thanks for the feedback! Last time I had similar conversation (with Riot Games API wrapper developers) they actually had the opposite opinion - that it's better for both library devs and end-user to just follow "bad naming/practices/models" from the original devs instead of rethinking/over-cooking. The proto-models are here and kinda working - why not just give easy access to them, they aren't so bad, honestly. Maybe affected by those, I just thought
But I definitely see your points/philosophy. Sorry that I was too linear. Let's modelize this stuff right away then. So... I've done a total overhaul of the PR (maybe I should resubmit it for cleaner commit history?). This is what I changed:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks very good, nearly there.
Last time I had similar conversation (with Riot Games API wrapper developers) they actually had the opposite opinion - that it's better for both library devs and end-user to just follow "bad naming/practices/models" from the original devs instead of rethinking/over-cooking
I wouldn't disagree if the names were consistent at the very least but with steam they aren't
Co-authored-by: James Hilton-Balfe <gobot1234yt@gmail.com>
Black didn't automate it cause fmt: off 😒custom rule-set when
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Idk, is it too bad if I dont title stuff properly
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
and a few brush ups
for more information, see https://pre-commit.ci
…m.py into introduce-ext.dota2
Also got rid of the folder `models`; it was a bad idea.
for more information, see https://pre-commit.ci
Protobufs were updated :x
also fix for OD type 🤦♀️
🐸 Summary
This PR introduces
ext.dota2
extension.🖍️ TODO
🐍 Migrate ValvePython/dota2 implemented methods.
request_gc_profile(disabled by valve)Sorry, I'm not interested in implementing this atm.
Same^
Same^
🔚 Checklist